Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only fetch @remote_coverage_tools when collecting coverage #16995

Closed

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Dec 12, 2022

Before this change, every test rule had an implicit dependency on @bazel_tools//tools/test:coverage_report_generator, even though this tool is only used when collecting coverage.

This is fixed by moving it to CoverageOptions and using the late-bound default resolver to only create a dependency if coverage is enabled.

Also adds CoverageOptions to the set of options classes trimmed by --trim_test_configuration so that, as before, changing --coverage_report_generator doesn't cause non-test rules to be reanalyzed. This behavior now extends to --coverage_output_generator.

Fixes #15088

@fmeum
Copy link
Collaborator Author

fmeum commented Dec 12, 2022

Stacked on #16994, which should be merged first.

@fmeum fmeum force-pushed the 15088-cc-test-optional-coverage branch from 841bd5e to 986246f Compare December 12, 2022 15:52
@fmeum fmeum marked this pull request as ready for review December 12, 2022 15:52
@fmeum fmeum requested a review from lberki as a code owner December 12, 2022 15:52
@fmeum
Copy link
Collaborator Author

fmeum commented Dec 12, 2022

@c-mita Could you review?

@fmeum
Copy link
Collaborator Author

fmeum commented Dec 12, 2022

@bazel-io flag

(flagging for 6.1.0, not a regression)

@bazel-io bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Dec 12, 2022
@fmeum fmeum force-pushed the 15088-cc-test-optional-coverage branch 2 times, most recently from 0f7a97e to 78a53bf Compare December 12, 2022 16:14
@lberki
Copy link
Contributor

lberki commented Dec 12, 2022

cc @metti

@lberki lberki requested a review from comius December 12, 2022 16:33
@fmeum fmeum force-pushed the 15088-cc-test-optional-coverage branch from 78a53bf to 7b6d7f5 Compare December 13, 2022 08:35
@fmeum
Copy link
Collaborator Author

fmeum commented Dec 13, 2022

No longer stacked.

@sgowroji sgowroji added team-Rules-CPP Issues for C++ rules awaiting-review PR is awaiting review from an assigned reviewer labels Dec 13, 2022
Before this change, every test rule had an implicit dependency on
`@bazel_tools//tools/test:coverage_report_generator`, even though this
tool is only used when collecting coverage.

This is fixed by moving it to `CoverageOptions` and using the late-bound
default resolver to only create a dependency if coverage is enabled.

Also adds `CoverageOptions` to the set of options classes trimmed by
`--trim_test_configuration` so that, as before, changing
`--coverage_report_generator` doesn't cause non-test rules to be
reanalyzed. This behavior now extends to `--coverage_output_generator`.
@fmeum fmeum force-pushed the 15088-cc-test-optional-coverage branch from 7b6d7f5 to aec5d92 Compare December 13, 2022 08:58
@meteorcloudy
Copy link
Member

@bazel-io fork 6.1.0

@bazel-io bazel-io removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Dec 14, 2022
@comius comius added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Dec 27, 2022
@copybara-service copybara-service bot closed this in 4d188a9 Jan 2, 2023
@sgowroji sgowroji removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Jan 2, 2023
@fmeum fmeum deleted the 15088-cc-test-optional-coverage branch January 2, 2023 06:48
hvadehra pushed a commit that referenced this pull request Feb 14, 2023
Before this change, every test rule had an implicit dependency on `@bazel_tools//tools/test:coverage_report_generator`, even though this tool is only used when collecting coverage.

This is fixed by moving it to `CoverageOptions` and using the late-bound default resolver to only create a dependency if coverage is enabled.

Also adds `CoverageOptions` to the set of options classes trimmed by `--trim_test_configuration` so that, as before, changing `--coverage_report_generator` doesn't cause non-test rules to be reanalyzed. This behavior now extends to `--coverage_output_generator`.

Fixes #15088

Closes #16995.

PiperOrigin-RevId: 498949871
Change-Id: I2440fae2655bbb701e918ee2aa7acb008d8f97ed
fmeum added a commit to fmeum/bazel that referenced this pull request Feb 16, 2023
Before this change, every test rule had an implicit dependency on `@bazel_tools//tools/test:coverage_report_generator`, even though this tool is only used when collecting coverage.

This is fixed by moving it to `CoverageOptions` and using the late-bound default resolver to only create a dependency if coverage is enabled.

Also adds `CoverageOptions` to the set of options classes trimmed by `--trim_test_configuration` so that, as before, changing `--coverage_report_generator` doesn't cause non-test rules to be reanalyzed. This behavior now extends to `--coverage_output_generator`.

Fixes bazelbuild#15088

Closes bazelbuild#16995.

PiperOrigin-RevId: 498949871
Change-Id: I2440fae2655bbb701e918ee2aa7acb008d8f97ed
keertk added a commit that referenced this pull request Feb 19, 2023
Before this change, every test rule had an implicit dependency on `@bazel_tools//tools/test:coverage_report_generator`, even though this tool is only used when collecting coverage.

This is fixed by moving it to `CoverageOptions` and using the late-bound default resolver to only create a dependency if coverage is enabled.

Also adds `CoverageOptions` to the set of options classes trimmed by `--trim_test_configuration` so that, as before, changing `--coverage_report_generator` doesn't cause non-test rules to be reanalyzed. This behavior now extends to `--coverage_output_generator`.

Fixes #15088

Closes #16995.

PiperOrigin-RevId: 498949871
Change-Id: I2440fae2655bbb701e918ee2aa7acb008d8f97ed

Co-authored-by: kshyanashree <[email protected]>
Co-authored-by: keertk <[email protected]>
keertk added a commit that referenced this pull request Feb 19, 2023
…7287)

* Only fetch @remote_coverage_tools when collecting coverage

Before this change, every test rule had an implicit dependency on `@bazel_tools//tools/test:coverage_report_generator`, even though this tool is only used when collecting coverage.

This is fixed by moving it to `CoverageOptions` and using the late-bound default resolver to only create a dependency if coverage is enabled.

Also adds `CoverageOptions` to the set of options classes trimmed by `--trim_test_configuration` so that, as before, changing `--coverage_report_generator` doesn't cause non-test rules to be reanalyzed. This behavior now extends to `--coverage_output_generator`.

Fixes #15088

Closes #16995.

PiperOrigin-RevId: 498949871
Change-Id: I2440fae2655bbb701e918ee2aa7acb008d8f97ed

* Update BUILD

---------

Co-authored-by: Fabian Meumertzheim <[email protected]>
Co-authored-by: keertk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Rules-CPP Issues for C++ rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cc_test rules trigger remote JDK download (~200 MB)
6 participants